Conversation
Johan-Liebert1
left a comment
There was a problem hiding this comment.
We shouldn't outright remove this. This should be OR'd with something like ConditionKernelCommandLine=composefs
875f0ab to
d7a0967
Compare
Done. |
Johan-Liebert1
left a comment
There was a problem hiding this comment.
#2248 adds another kernel cmdline for composefs so we should not forget to add that in here as well once it's merged
|
Also, @godvino could you sign your commit with |
d7a0967 to
636f146
Compare
Done. |
|
Hmm... DCO's still failing. That command should've added a Do you have name and email properly set up for git? |
… units/timers. This file is created by ostree backend and won't be present for systems using composefs backend Signed-off-by: Godvin Ouseph <godvvino@gmail.com>
636f146 to
781bbca
Compare
Should be fixed now. |
| Documentation=man:bootc(8) | ||
| ConditionPathExists=/run/ostree-booted | ||
| ConditionPathExists=|/run/ostree-booted | ||
| ConditionKernelCommandLine=|composefs |
There was a problem hiding this comment.
So this is a severe oversight on our part, and part of this is that we were just incrementally getting the tests passing for composefs and didn't have a conformance suite for this.
However I now have a serious worry that fixing this could seriously break people who were using the composefs backend and basically relying on us not doing this (i.e. they have their own update systems).
Also of note, this change will also break after #2248 lands in the general case.
If we have to I would argue that we add /run/composefs-booted, I think that's been discussed elsewhere.
(Though strictly speaking a better check is for the source of /'s mount containing composefs perhaps, that's what we do in bootc's generator itself)
There was a problem hiding this comment.
Also, I now kind of regret shipping the builtin timer. It wasn't...wrong exactly but it's too simplistic and basically everyone needs to replace it, and if they need to do that then we shouldn't have had it on at all...
There was a problem hiding this comment.
Maybe we can disable this timer by default and people who want it can enable it?
There was a problem hiding this comment.
It'd be a very breaking change to disable the timer for the ostree backend too.
I am not seeing an easy path out of this situation without VERY LOUD warnings on either side.
OK, one technical thing we could do: look at the systemd journal and see whether any previous boots had something else do an update but...so heuristic and messy, I am not sure it'd be worth it.
cgwalters
left a comment
There was a problem hiding this comment.
Per above, at least this needs some more consideration
This file is created by ostree backend and won't be present for systems using composefs backend